A random-random test for time-series data #132556

pabloem · 2025-08-07T23:58:36Z

Follow up items after this PR:

Test rate function and counters in general
Randomize window size
Increase test size
Hit some more corner cases (e.g. zero out some parameters)

elasticsearchmachine · 2025-08-11T18:08:31Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

not-napoleon

I'd like to get some confirmation from @kkrik-es that this is doing what he wants, but I think it's pretty good. I left some feedback, none of which is critical but I'd like to get it addressed.

not-napoleon · 2025-08-11T18:21:48Z

test/framework/src/main/java/org/elasticsearch/datageneration/FieldType.java

@@ -78,6 +79,7 @@ public FieldDataGenerator generator(String fieldName, DataSource dataSource) {
            case IP -> new IpFieldDataGenerator(dataSource);
            case CONSTANT_KEYWORD -> new ConstantKeywordFieldDataGenerator();
            case WILDCARD -> new WildcardFieldDataGenerator(dataSource);
+            default -> null;


I would prefer not to have a default in this switch. When someone adds a new FieldType here, I want the compiler to tell them they must also update this switch, but the default will hide that.

In fact, this probably shouldn't be a switch. It should probably be an abstract method or a function member on the enum itself. Switches on enums are a bit of a smell, and switches on enums from within the enum itself are so smelly as to almost be an error, IMHO. I realize you didn't add this switch, but this is a good opportunity to refactor it and leave it better than we found it.

maybe we can compromise on removing the default? 😅

test/framework/src/main/java/org/elasticsearch/datageneration/MappingGenerator.java

...c/main/java/org/elasticsearch/datageneration/datasource/DefaultMappingParametersHandler.java

not-napoleon · 2025-08-11T18:43:28Z

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

+    private List<XContentBuilder> documents = null;
+    private DataGenerationHelper dataGenerationHelper;
+
+    private static final class DataGenerationHelper {


I wonder if this should be a top level class. Seems like we'll want to build multiple test classes using this framework.

I've moved this class to its own file! TY.

not-napoleon · 2025-08-11T18:50:17Z

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

+        private static Object randomDimensionValue(String dimensionName) {
+            // We use dimensionName to determine the type of the value.
+            var isNumeric = dimensionName.hashCode() % 5 == 0;
+            if (isNumeric) {


What about IP dimensions?

added 20% of dimensions as IP-like.

as follow up ill add dynamic mapping to parse as ip. thoughts?

Why not rely on the existing one:
https://github.com/elastic/elasticsearch/blob/main/test/framework/src/main/java/org/elasticsearch/datageneration/fields/leaf/IpFieldDataGenerator.java

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

pabloem

TY @not-napoleon - ptal!

test/framework/src/main/java/org/elasticsearch/datageneration/MappingGenerator.java

...c/main/java/org/elasticsearch/datageneration/datasource/DefaultMappingParametersHandler.java

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

pabloem · 2025-08-12T00:03:53Z

test/framework/src/main/java/org/elasticsearch/datageneration/FieldType.java

@@ -78,6 +79,7 @@ public FieldDataGenerator generator(String fieldName, DataSource dataSource) {
            case IP -> new IpFieldDataGenerator(dataSource);
            case CONSTANT_KEYWORD -> new ConstantKeywordFieldDataGenerator();
            case WILDCARD -> new WildcardFieldDataGenerator(dataSource);
+            default -> null;


maybe we can compromise on removing the default? 😅

pabloem · 2025-08-12T00:04:23Z

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

+    private List<XContentBuilder> documents = null;
+    private DataGenerationHelper dataGenerationHelper;
+
+    private static final class DataGenerationHelper {


I've moved this class to its own file! TY.

pabloem · 2025-08-12T00:06:49Z

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

+        private static Object randomDimensionValue(String dimensionName) {
+            // We use dimensionName to determine the type of the value.
+            var isNumeric = dimensionName.hashCode() % 5 == 0;
+            if (isNumeric) {


added 20% of dimensions as IP-like.

pabloem · 2025-08-12T00:17:43Z

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

+        private static Object randomDimensionValue(String dimensionName) {
+            // We use dimensionName to determine the type of the value.
+            var isNumeric = dimensionName.hashCode() % 5 == 0;
+            if (isNumeric) {


as follow up ill add dynamic mapping to parse as ip. thoughts?

...rc/main/java/org/elasticsearch/datageneration/datasource/DefaultObjectGenerationHandler.java

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

kkrik-es · 2025-08-12T07:57:22Z

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

+                var docValues = windowDataPoints.stream()
+                    .map(doc -> ((Map<String, Integer>) doc.get("metrics")).get("gauge_hdd.bytes.used"))
+                    .toList();
+                // Verify that the first column is the max value (the query gets max, avg, min in that order)


In a follow-up, you can avoid hard-coding by defining an enum for each fuction, with corresponding validation logic.

kkrik-es · 2025-08-12T07:58:45Z

...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/GenerativeTSIT.java

+                // Verify that the second column is the avg value (thus why row.get(2))
+                docValues.stream().mapToDouble(Integer::doubleValue).average().ifPresentOrElse(avgValue -> {
+                    var res = (Double) row.get(2);
+                    assertThat(res, closeTo(avgValue, res * 0.5));


Why do we need the 0.5 factor?

that was a mistake (I meant to do 5% not 50%).
However, average calculation does seem to have up to 20-25% error between ES and test-framework numbers. Should I check if that's a bug and how to deal with it?

There should be no error here (Double.compare() should return 0), so there's a bug somewhere. Let's investigate separately.

kkrik-es · 2025-08-12T08:00:35Z

...src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TSDataGenerationHelper.java

+
+    private static Object randomDimensionValue(String dimensionName) {
+        // We use dimensionName to determine the type of the value.
+        var isNumeric = dimensionName.hashCode() % 5 == 0;


Nit: let's just use randomDouble() < 0.2. Same below.

I see, this needs to be consistent per field. Why not rely on the data generation framework to provide these values? You can start simple with all dimensions being keyword fields - not dynamic, no pass-through subfields.

I tried to closely match an opentelemetry use case, so a bunch of dynamic dimensions under attributes... Do you think that's problematic? I can also add a non-dynamic test case as follow up. Thoughts?

It's not problematic but complicates things somewhat.. We need it eventually, I was thinking whether we should start with something simpler. Up to you, this works as well.

...src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TSDataGenerationHelper.java

kkrik-es

Thanks Pablo, this ia a good step. It's nice that you tried to include the pass-through field on the first take, though that complicates things somewhat. I'd start with statically defined dimension and metric fields to get the validation logic in place first, then introduce dynamic fields on top of that.

Let's try to refactor the logic slightly so that it can be further extended in follow-up PRs.

kkrik-es · 2025-08-13T07:45:29Z

...src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/RandomizedTimeSeriesIT.java

+        return List.of(
+            DataStreamsPlugin.class,
+            LocalStateCompositeXPackPlugin.class,
+            // Downsample.class, // TODO(pabloem): What are these


kkrik-es

Looks good, thanks for addressing the comments. Let's keep iterating.

First test case in prototype messy test file

613bda8

elasticsearchmachine added the v9.2.0 label Aug 7, 2025

elasticsearchmachine and others added 6 commits August 8, 2025 00:04

[CI] Auto commit changes from spotless

2d70cef

First two randomized test cases

19e2f8a

smol cleanup

14b4c17

Merge branch 'main' into pem-randomrandom-testing

836d49a

[CI] Auto commit changes from spotless

64624be

cleanup and ready for first check

77a368b

pabloem marked this pull request as ready for review August 11, 2025 18:06

Merge branch 'main' into pem-randomrandom-testing

f53f904

pabloem changed the title ~~[wip][do not review] A random-random test for time-series data~~ A random-random test for time-series data Aug 11, 2025

elasticsearchmachine added the needs:triage Requires assignment of a team area label label Aug 11, 2025

pabloem added >test Issues or PRs that are addressing/adding tests :StorageEngine/TSDB You know, for Metrics :StorageEngine/ES|QL Timeseries / metrics / logsdb capabilities in ES|QL and removed needs:triage Requires assignment of a team area label labels Aug 11, 2025

elasticsearchmachine added the Team:StorageEngine label Aug 11, 2025

not-napoleon reviewed Aug 11, 2025

View reviewed changes

Address comments

afdac85

pabloem commented Aug 12, 2025

View reviewed changes